[libc++] Require fancy pointer to be correctly convertible #152989
+99
−114
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesThis patch removes a bunch of code that works around incorrect implementations of fancy pointers. It's possible that this breaks user code. However, any breakage is likely to catch serious latent bugs. Patch is 34.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152989.diff 5 Files Affected:
diff --git a/libcxx/include/__hash_table b/libcxx/include/__hash_table
index dacc152030e14..dbc41fdf0f459 100644
--- a/libcxx/include/__hash_table
+++ b/libcxx/include/__hash_table
@@ -96,7 +96,7 @@ struct __hash_node_base {
__next_pointer __next_;
_LIBCPP_HIDE_FROM_ABI __next_pointer __ptr() _NOEXCEPT {
- return static_cast<__next_pointer>(pointer_traits<__node_base_pointer>::pointer_to(*this));
+ return pointer_traits<__node_base_pointer>::pointer_to(*this);
}
_LIBCPP_HIDE_FROM_ABI __node_pointer __upcast() _NOEXCEPT {
@@ -1499,9 +1499,9 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_insert_multi(const_iterator __p
while (__pp->__next_ != __np)
__pp = __pp->__next_;
__cp->__next_ = __np;
- __pp->__next_ = static_cast<__next_pointer>(__cp);
+ __pp->__next_ = __cp;
++size();
- return iterator(static_cast<__next_pointer>(__cp));
+ return iterator(__cp);
}
return __node_insert_multi(__cp);
}
@@ -1547,9 +1547,9 @@ __hash_table<_Tp, _Hash, _Equal, _Alloc>::__emplace_unique_key_args(_Key const&
__bucket_list_[std::__constrain_hash(__h->__next_->__hash(), __bc)] = __h.get()->__ptr();
} else {
__h->__next_ = __pn->__next_;
- __pn->__next_ = static_cast<__next_pointer>(__h.get());
+ __pn->__next_ = __h.get();
}
- __nd = static_cast<__next_pointer>(__h.release());
+ __nd = __h.release();
// increment size
++size();
__inserted = true;
diff --git a/libcxx/include/__memory/pointer_traits.h b/libcxx/include/__memory/pointer_traits.h
index 8c7f8dff1b76b..336422befb6ee 100644
--- a/libcxx/include/__memory/pointer_traits.h
+++ b/libcxx/include/__memory/pointer_traits.h
@@ -252,18 +252,6 @@ concept __resettable_smart_pointer_with_args = requires(_Smart __s, _Pointer __p
#endif
-// This function ensures safe conversions between fancy pointers at compile-time, where we avoid casts from/to
-// `__void_pointer` by obtaining the underlying raw pointer from the fancy pointer using `std::to_address`,
-// then dereferencing it to retrieve the pointed-to object, and finally constructing the target fancy pointer
-// to that object using the `std::pointer_traits<>::pinter_to` function.
-template <class _PtrTo, class _PtrFrom>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI _PtrTo __static_fancy_pointer_cast(const _PtrFrom& __p) {
- using __ptr_traits = pointer_traits<_PtrTo>;
- using __element_type = typename __ptr_traits::element_type;
- return __p ? __ptr_traits::pointer_to(*static_cast<__element_type*>(std::addressof(*__p)))
- : static_cast<_PtrTo>(nullptr);
-}
-
_LIBCPP_END_NAMESPACE_STD
_LIBCPP_POP_MACROS
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 6dadd0915c984..b804a542bbd8d 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -171,10 +171,10 @@ template <class _EndNodePtr, class _NodePtr>
inline _LIBCPP_HIDE_FROM_ABI _EndNodePtr __tree_next_iter(_NodePtr __x) _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(__x != nullptr, "node shouldn't be null");
if (__x->__right_ != nullptr)
- return static_cast<_EndNodePtr>(std::__tree_min(__x->__right_));
+ return std::__tree_min(__x->__right_);
while (!std::__tree_is_left_child(__x))
__x = __x->__parent_unsafe();
- return static_cast<_EndNodePtr>(__x->__parent_);
+ return __x->__parent_;
}
// Returns: pointer to the previous in-order node before __x.
@@ -541,7 +541,7 @@ public:
_LIBCPP_HIDE_FROM_ABI pointer __parent_unsafe() const { return static_cast<pointer>(__parent_); }
- _LIBCPP_HIDE_FROM_ABI void __set_parent(pointer __p) { __parent_ = static_cast<__end_node_pointer>(__p); }
+ _LIBCPP_HIDE_FROM_ABI void __set_parent(pointer __p) { __parent_ = __p; }
~__tree_node_base() = delete;
__tree_node_base(__tree_node_base const&) = delete;
@@ -635,7 +635,7 @@ public:
}
_LIBCPP_HIDE_FROM_ABI __tree_iterator& operator--() {
- __ptr_ = static_cast<__end_node_pointer>(std::__tree_prev_iter<__node_base_pointer>(__ptr_));
+ __ptr_ = std::__tree_prev_iter<__node_base_pointer>(__ptr_);
return *this;
}
_LIBCPP_HIDE_FROM_ABI __tree_iterator operator--(int) {
@@ -698,7 +698,7 @@ public:
}
_LIBCPP_HIDE_FROM_ABI __tree_const_iterator& operator--() {
- __ptr_ = static_cast<__end_node_pointer>(std::__tree_prev_iter<__node_base_pointer>(__ptr_));
+ __ptr_ = std::__tree_prev_iter<__node_base_pointer>(__ptr_);
return *this;
}
@@ -1212,12 +1212,12 @@ private:
__node_pointer __new_node_ptr = __new_node.release();
__new_node_ptr->__is_black_ = __src->__is_black_;
- __new_node_ptr->__left_ = static_cast<__node_base_pointer>(__left.release());
- __new_node_ptr->__right_ = static_cast<__node_base_pointer>(__right);
+ __new_node_ptr->__left_ = __left.release();
+ __new_node_ptr->__right_ = __right;
if (__new_node_ptr->__left_)
- __new_node_ptr->__left_->__parent_ = static_cast<__end_node_pointer>(__new_node_ptr);
+ __new_node_ptr->__left_->__parent_ = __new_node_ptr;
if (__new_node_ptr->__right_)
- __new_node_ptr->__right_->__parent_ = static_cast<__end_node_pointer>(__new_node_ptr);
+ __new_node_ptr->__right_->__parent_ = __new_node_ptr;
return __new_node_ptr;
}
@@ -1239,24 +1239,24 @@ private:
// If we already have a left node in the destination tree, reuse it and copy-assign recursively
if (__dest->__left_) {
- __dest->__left_ = static_cast<__node_base_pointer>(__copy_assign_tree(
- static_cast<__node_pointer>(__dest->__left_), static_cast<__node_pointer>(__src->__left_)));
+ __dest->__left_ =
+ __copy_assign_tree(static_cast<__node_pointer>(__dest->__left_), static_cast<__node_pointer>(__src->__left_));
// Otherwise, we must create new nodes; copy-construct from here on
} else if (__src->__left_) {
auto __new_left = __copy_construct_tree(static_cast<__node_pointer>(__src->__left_));
- __dest->__left_ = static_cast<__node_base_pointer>(__new_left);
- __new_left->__parent_ = static_cast<__end_node_pointer>(__dest);
+ __dest->__left_ = __new_left;
+ __new_left->__parent_ = __dest;
}
// Identical to the left case above, just for the right nodes
if (__dest->__right_) {
- __dest->__right_ = static_cast<__node_base_pointer>(__copy_assign_tree(
- static_cast<__node_pointer>(__dest->__right_), static_cast<__node_pointer>(__src->__right_)));
+ __dest->__right_ = __copy_assign_tree(
+ static_cast<__node_pointer>(__dest->__right_), static_cast<__node_pointer>(__src->__right_));
} else if (__src->__right_) {
auto __new_right = __copy_construct_tree(static_cast<__node_pointer>(__src->__right_));
- __dest->__right_ = static_cast<__node_base_pointer>(__new_right);
- __new_right->__parent_ = static_cast<__end_node_pointer>(__dest);
+ __dest->__right_ = __new_right;
+ __new_right->__parent_ = __dest;
}
return __dest;
@@ -1332,15 +1332,14 @@ __tree<_Tp, _Compare, _Allocator>& __tree<_Tp, _Compare, _Allocator>::operator=(
__copy_assign_alloc(__t);
if (__size_ != 0) {
- *__root_ptr() = static_cast<__node_base_pointer>(__copy_assign_tree(__root(), __t.__root()));
+ *__root_ptr() = __copy_assign_tree(__root(), __t.__root());
} else {
- *__root_ptr() = static_cast<__node_base_pointer>(__copy_construct_tree(__t.__root()));
+ *__root_ptr() = __copy_construct_tree(__t.__root());
if (__root())
__root()->__parent_ = __end_node();
}
- __begin_node_ =
- __end_node()->__left_ ? static_cast<__end_node_pointer>(std::__tree_min(__end_node()->__left_)) : __end_node();
- __size_ = __t.size();
+ __begin_node_ = __end_node()->__left_ ? std::__tree_min(__end_node()->__left_) : __end_node();
+ __size_ = __t.size();
return *this;
}
@@ -1394,9 +1393,9 @@ __tree<_Tp, _Compare, _Allocator>::__tree(const __tree& __t)
if (__t.size() == 0)
return;
- *__root_ptr() = static_cast<__node_base_pointer>(__copy_construct_tree(__t.__root()));
+ *__root_ptr() = __copy_construct_tree(__t.__root());
__root()->__parent_ = __end_node();
- __begin_node_ = static_cast<__end_node_pointer>(std::__tree_min(__end_node()->__left_));
+ __begin_node_ = std::__tree_min(__end_node()->__left_);
__size_ = __t.size();
}
@@ -1411,7 +1410,7 @@ __tree<_Tp, _Compare, _Allocator>::__tree(__tree&& __t) _NOEXCEPT_(
if (__size_ == 0)
__begin_node_ = __end_node();
else {
- __end_node()->__left_->__parent_ = static_cast<__end_node_pointer>(__end_node());
+ __end_node()->__left_->__parent_ = __end_node();
__t.__begin_node_ = __t.__end_node();
__t.__end_node()->__left_ = nullptr;
__t.__size_ = 0;
@@ -1427,7 +1426,7 @@ __tree<_Tp, _Compare, _Allocator>::__tree(__tree&& __t, const allocator_type& __
else {
__begin_node_ = __t.__begin_node_;
__end_node()->__left_ = __t.__end_node()->__left_;
- __end_node()->__left_->__parent_ = static_cast<__end_node_pointer>(__end_node());
+ __end_node()->__left_->__parent_ = __end_node();
__size_ = __t.__size_;
__t.__begin_node_ = __t.__end_node();
__t.__end_node()->__left_ = nullptr;
@@ -1450,7 +1449,7 @@ void __tree<_Tp, _Compare, _Allocator>::__move_assign(__tree& __t, true_type)
if (__size_ == 0)
__begin_node_ = __end_node();
else {
- __end_node()->__left_->__parent_ = static_cast<__end_node_pointer>(__end_node());
+ __end_node()->__left_->__parent_ = __end_node();
__t.__begin_node_ = __t.__end_node();
__t.__end_node()->__left_ = nullptr;
__t.__size_ = 0;
@@ -1545,14 +1544,14 @@ __tree<_Tp, _Compare, _Allocator>::__find_leaf_low(__end_node_pointer& __parent,
if (__nd->__right_ != nullptr)
__nd = static_cast<__node_pointer>(__nd->__right_);
else {
- __parent = static_cast<__end_node_pointer>(__nd);
+ __parent = __nd;
return __nd->__right_;
}
} else {
if (__nd->__left_ != nullptr)
__nd = static_cast<__node_pointer>(__nd->__left_);
else {
- __parent = static_cast<__end_node_pointer>(__nd);
+ __parent = __nd;
return __parent->__left_;
}
}
@@ -1575,14 +1574,14 @@ __tree<_Tp, _Compare, _Allocator>::__find_leaf_high(__end_node_pointer& __parent
if (__nd->__left_ != nullptr)
__nd = static_cast<__node_pointer>(__nd->__left_);
else {
- __parent = static_cast<__end_node_pointer>(__nd);
+ __parent = __nd;
return __parent->__left_;
}
} else {
if (__nd->__right_ != nullptr)
__nd = static_cast<__node_pointer>(__nd->__right_);
else {
- __parent = static_cast<__end_node_pointer>(__nd);
+ __parent = __nd;
return __nd->__right_;
}
}
@@ -1608,10 +1607,10 @@ typename __tree<_Tp, _Compare, _Allocator>::__node_base_pointer& __tree<_Tp, _Co
if (__prior == begin() || !value_comp()(__v, *--__prior)) {
// *prev(__hint) <= __v <= *__hint
if (__hint.__ptr_->__left_ == nullptr) {
- __parent = static_cast<__end_node_pointer>(__hint.__ptr_);
+ __parent = __hint.__ptr_;
return __parent->__left_;
} else {
- __parent = static_cast<__end_node_pointer>(__prior.__ptr_);
+ __parent = __prior.__ptr_;
return static_cast<__node_base_pointer>(__prior.__ptr_)->__right_;
}
}
@@ -1639,7 +1638,7 @@ __tree<_Tp, _Compare, _Allocator>::__find_equal(__end_node_pointer& __parent, co
__nd_ptr = std::addressof(__nd->__left_);
__nd = static_cast<__node_pointer>(__nd->__left_);
} else {
- __parent = static_cast<__end_node_pointer>(__nd);
+ __parent = __nd;
return __parent->__left_;
}
} else if (value_comp()(__nd->__value_, __v)) {
@@ -1647,11 +1646,11 @@ __tree<_Tp, _Compare, _Allocator>::__find_equal(__end_node_pointer& __parent, co
__nd_ptr = std::addressof(__nd->__right_);
__nd = static_cast<__node_pointer>(__nd->__right_);
} else {
- __parent = static_cast<__end_node_pointer>(__nd);
+ __parent = __nd;
return __nd->__right_;
}
} else {
- __parent = static_cast<__end_node_pointer>(__nd);
+ __parent = __nd;
return *__nd_ptr;
}
}
@@ -1719,7 +1718,7 @@ void __tree<_Tp, _Compare, _Allocator>::__insert_node_at(
// __new_node->__is_black_ is initialized in __tree_balance_after_insert
__child = __new_node;
if (__begin_node_->__left_ != nullptr)
- __begin_node_ = static_cast<__end_node_pointer>(__begin_node_->__left_);
+ __begin_node_ = __begin_node_->__left_;
std::__tree_balance_after_insert(__end_node()->__left_, __child);
++__size_;
}
@@ -1734,7 +1733,7 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_unique_key_args(_Key const& __k, _A
bool __inserted = false;
if (__child == nullptr) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
+ __insert_node_at(__parent, __child, __h.get());
__r = __h.release();
__inserted = true;
}
@@ -1753,7 +1752,7 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_key_args(
bool __inserted = false;
if (__child == nullptr) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
+ __insert_node_at(__parent, __child, __h.get());
__r = __h.release();
__inserted = true;
}
@@ -1781,7 +1780,7 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_unique_impl(_Args&&... __args) {
__node_pointer __r = static_cast<__node_pointer>(__child);
bool __inserted = false;
if (__child == nullptr) {
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
+ __insert_node_at(__parent, __child, __h.get());
__r = __h.release();
__inserted = true;
}
@@ -1798,7 +1797,7 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_unique_impl(const_iterator __p
__node_base_pointer& __child = __find_equal(__p, __parent, __dummy, __h->__value_);
__node_pointer __r = static_cast<__node_pointer>(__child);
if (__child == nullptr) {
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
+ __insert_node_at(__parent, __child, __h.get());
__r = __h.release();
}
return iterator(__r);
@@ -1811,8 +1810,8 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_multi(_Args&&... __args) {
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
__end_node_pointer __parent;
__node_base_pointer& __child = __find_leaf_high(__parent, __h->__value_);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
- return iterator(static_cast<__node_pointer>(__h.release()));
+ __insert_node_at(__parent, __child, __h.get());
+ return iterator(__h.release());
}
template <class _Tp, class _Compare, class _Allocator>
@@ -1822,8 +1821,8 @@ __tree<_Tp, _Compare, _Allocator>::__emplace_hint_multi(const_iterator __p, _Arg
__node_holder __h = __construct_node(std::forward<_Args>(__args)...);
__end_node_pointer __parent;
__node_base_pointer& __child = __find_leaf(__p, __parent, __h->__value_);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__h.get()));
- return iterator(static_cast<__node_pointer>(__h.release()));
+ __insert_node_at(__parent, __child, __h.get());
+ return iterator(__h.release());
}
template <class _Tp, class _Compare, class _Allocator>
@@ -1835,7 +1834,7 @@ __tree<_Tp, _Compare, _Allocator>::__node_assign_unique(const value_type& __v, _
bool __inserted = false;
if (__child == nullptr) {
__assign_value(__nd->__value_, __v);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__nd));
+ __insert_node_at(__parent, __child, __nd);
__r = __nd;
__inserted = true;
}
@@ -1847,7 +1846,7 @@ typename __tree<_Tp, _Compare, _Allocator>::iterator
__tree<_Tp, _Compare, _Allocator>::__node_insert_multi(__node_pointer __nd) {
__end_node_pointer __parent;
__node_base_pointer& __child = __find_leaf_high(__parent, __nd->__value_);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__nd));
+ __insert_node_at(__parent, __child, __nd);
return iterator(__nd);
}
@@ -1856,7 +1855,7 @@ typename __tree<_Tp, _Compare, _Allocator>::iterator
__tree<_Tp, _Compare, _Allocator>::__node_insert_multi(const_iterator __p, __node_pointer __nd) {
__end_node_pointer __parent;
__node_base_pointer& __child = __find_leaf(__p, __parent, __nd->__value_);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__nd));
+ __insert_node_at(__parent, __child, __nd);
return iterator(__nd);
}
@@ -1886,7 +1885,7 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_unique(_NodeHandle&& __n
if (__child != nullptr)
return _InsertReturnType{iterator(static_cast<__node_pointer>(__child)), false, std::move(__nh)};
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__ptr));
+ __insert_node_at(__parent, __child, __ptr);
__nh.__release_ptr();
return _InsertReturnType{iterator(__ptr), true, _NodeHandle()};
}
@@ -1904,7 +1903,7 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_unique(const_iterator __
__node_base_pointer& __child = __find_equal(__hint, __parent, __dummy, __ptr->__value_);
__node_pointer __r = static_cast<__node_pointer>(__child);
if (__child == nullptr) {
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__ptr));
+ __insert_node_at(__parent, __child, __ptr);
__r = __ptr;
__nh.__release_ptr();
}
@@ -1941,7 +1940,7 @@ _LIBCPP_HIDE_FROM_ABI void __tree<_Tp, _Compare, _Allocator>::__node_handle_merg
if (__child != nullptr)
continue;
__source.__remove_node_pointer(__src_ptr);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__src_ptr));
+ __insert_node_at(__parent, __child, __src_ptr);
}
}
@@ -1954,7 +1953,7 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_multi(_NodeHandle&& __nh
__node_pointer __ptr = __nh.__ptr_;
__end_node_pointer __parent;
__node_base_pointer& __child = __find_leaf_high(__parent, __ptr->__value_);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__ptr));
+ __insert_node_at(__parent, __child, __ptr);
__nh.__release_ptr();
return iterator(__ptr);
}
@@ -1969,7 +1968,7 @@ __tree<_Tp, _Compare, _Allocator>::__node_handle_insert_multi(const_iterator __h
__node_pointer __ptr = __nh.__ptr_;
__end_node_pointer __parent;
__node_base_pointer& __child = __find_leaf(__hint, __parent, __ptr->__value_);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__ptr));
+ __insert_node_at(__parent, __child, __ptr);
__nh.__release_ptr();
return iterator(__ptr);
}
@@ -1985,7 +1984,7 @@ _LIBCPP_HIDE_FROM_ABI void __tree<_Tp, _Compare, _Allocator>::__node_handle_merg
__node_base_pointer& __child = __find_leaf_high(__parent, __src_ptr->__value_);
++__i;
__source.__remove_node_pointer(__src_ptr);
- __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__src_ptr));
+ __insert_node_at(__parent, __child, __src_ptr);
}
}
@@ -2074,13 +2073,13 @@ __tree<_Tp, _Compare, _Allocator>::__count_multi(const _Key& __k) const {
__node_pointer __rt = __root();
while (__rt != nullptr) {
if (value_comp()(__k, __rt->__value_)) {
- __result = static_cast<__end_node_pointer>(__rt);
+ __result = __rt;
__rt = static_cast<__node_pointer>(__rt->__left_);
} else if (value_comp()(__rt->__value_, __k))
__rt = static_cast<__node_pointer>(__...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions ,h -- libcxx/include/__hash_table libcxx/include/__memory/pointer_traits.h libcxx/include/__tree libcxx/include/forward_list libcxx/test/support/min_allocator.hView the diff from clang-format here.diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index b804a542b..92377d6bf 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -2078,9 +2078,8 @@ __tree<_Tp, _Compare, _Allocator>::__count_multi(const _Key& __k) const {
} else if (value_comp()(__rt->__value_, __k))
__rt = static_cast<__node_pointer>(__rt->__right_);
else
- return std::distance(
- __lower_bound(__k, static_cast<__node_pointer>(__rt->__left_), __rt),
- __upper_bound(__k, static_cast<__node_pointer>(__rt->__right_), __result));
+ return std::distance(__lower_bound(__k, static_cast<__node_pointer>(__rt->__left_), __rt),
+ __upper_bound(__k, static_cast<__node_pointer>(__rt->__right_), __result));
}
return 0;
}
|
philnik777
force-pushed
the
require_correct_fancy_pointer_conversions
branch
from
19c6b7d to
9e739a7
Compare
|
This seems to require convertibility beyond the requirements in [allocator.requirements.general]... |
AFAICT that is completely silent on how to do up/down casts at all, which we already use. IMO we're providing a better user experience with this change, since we can remove |
ldionne
requested changes
Aug 13, 2025
There was a problem hiding this comment.
IMO this change would be reasonable, but I think we need a LWG issue to clarify whether the requirements in https://eel.is/c++draft/allocator.requirements.general are lacking, and if not, what implementers are expected to do.
| @@ -96,7 +96,7 @@ struct __hash_node_base { | |||
| __next_pointer __next_; | |||
There was a problem hiding this comment.
Please add a release note and mention the potential for breakage. In particular, what kind of code would break after this change and what users should do to fix it.
| @@ -96,7 +96,7 @@ struct __hash_node_base { | |||
| __next_pointer __next_; | |||
|
|
|||
| _LIBCPP_HIDE_FROM_ABI __next_pointer __ptr() _NOEXCEPT { | |||
| return static_cast<__next_pointer>(pointer_traits<__node_base_pointer>::pointer_to(*this)); | |||
| return pointer_traits<__node_base_pointer>::pointer_to(*this); | |||
There was a problem hiding this comment.
Do I understand correctly that this would be the "right" way to perform this cast if we only want to use the requirements in https://eel.is/c++draft/allocator.requirements.general ?
Suggested change
| return pointer_traits<__node_base_pointer>::pointer_to(*this); | |
| return static_cast<__next_pointer>(static_cast<void_pointer>(pointer_traits<__node_base_pointer>::pointer_to(*this))); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch removes a bunch of code that works around incorrect implementations of fancy pointers. It's possible that this breaks user code. However, any breakage is likely to catch serious latent bugs.